AO3-7182 Hide restricted works from guests on prompts page#5685
AO3-7182 Hide restricted works from guests on prompts page#5685luis-pabon-tf wants to merge 11 commits intootwcode:masterfrom
Conversation
sarken
left a comment
sarken
left a comment
There was a problem hiding this comment.
Hi, Luis -- it's great to see you again!
Thank you for working on this, and I'm sorry we didn't see your comment on the issue asking for clarification about the intended behavior. We'd like the existence of these restricted works completely hidden from guests, the same way we hide restricted works on, e.g., tag work pages.
What that means is:
- If the only fills for a prompt are restricted, the "Fulfilled By" listbox should not be displayed.
- If there are both public and restricted fills for a prompt, the "Fulfilled By" listbox should be present, but only list the public works.
If you could make that change, we'd appreciate it!
github-actions
Bot
added
Coder Has Actioned Review
and removed
Reviewed: Action Needed
labels
Thanks for the review! just made the change and am awaiting the automatic testing to verify it. |
sarken
left a comment
sarken
left a comment
There was a problem hiding this comment.
If you could also add a feature test, that would be great -- thank you!
| CollectionItem.user_approval_statuses[:approved], CollectionItem.collection_approval_statuses[:approved]) | ||
| } | ||
|
|
||
| scope :fulfilled_unrestricted, -> { fulfilled.where("works.restricted = 0") } |
There was a problem hiding this comment.
Let's not include the fulfilled scope and instead make it something we can chain onto it:
# TODO: AO3-6024 for making sure we also exclude works hidden by admin
scope :work_visible_to_all, -> { where("works.restricted = 0") }| def fulfilled_unrestricted_claims | ||
| self.request_claims.fulfilled_unrestricted | ||
| end |
There was a problem hiding this comment.
We can update this to use our chained scopes and also give it a name that's a little clearer (right now, it sounds like the claim rather than the fill is unrestricted):
def fulfilled_claims_visible_to_all
self.request_claims.fulfilled.work_visible_to_all
end|
|
||
| <% # if prompt has been fulfilled list works %> | ||
| <% unless prompt.unfulfilled? %> | ||
| <% unless prompt.unfulfilled? || (guest? && prompt.fulfilled_unrestricted_claims.count < 1) %> |
There was a problem hiding this comment.
I think we can avoid a count here and do prompt.fulfilled_claims_visible_to_all.empty?
Co-Authored-By: sarken <907055+sarken@users.noreply.github.com>
Co-Authored-By: sarken <907055+sarken@users.noreply.github.com>
github-actions
Bot
added
Coder Has Actioned Review
and removed
Reviewed: Action Needed
labels
sarken
left a comment
sarken
left a comment
There was a problem hiding this comment.
The code looks good! All we need now is a test.
github-actions
Bot
added
Coder Has Actioned Review
and removed
Reviewed: Action Needed
labels
Thanks for checking it! I just finished testing the new testand am awaiting the Github version of it to run. |
sarken
left a comment
sarken
left a comment
There was a problem hiding this comment.
Just a couple nitpicks, but otherwise it looks good-- thanks!
| And I go to "Battle 12" collection's page | ||
| And I follow "Prompts (" |
There was a problem hiding this comment.
I think you can use When I view prompts for "Battle 12" in place of these two steps.
| And I press "Post" | ||
| And I go to "Battle 12" collection's page | ||
| And I follow "Prompts (" | ||
| Then I should see "Fulfilled By" |
There was a problem hiding this comment.
Let's be extra thorough and also check for the work with And I should see "Restricted Fill" (and do the opposite at the end, with And I should not see "Restricted Fill")
| When I am logged in as "myname1" | ||
| When I sign up for Battle 12 | ||
| When I am logged in as "myname2" | ||
| And I claim a prompt from "Battle 12" | ||
| And I start to fulfill my claim with "Restricted Fill" | ||
| And I lock the work | ||
| And I press "Post" |
There was a problem hiding this comment.
These setup steps can all be And steps under the Given
2 participants
Pull Request Checklist
as the first thing in your pull request title (e.g.
AO3-1234 Fix thing)until they are reviewed and merged before creating new pull requests.
Issue
https://otwarchive.atlassian.net/browse/AO3-7182
Purpose
Fixes a visibility bug on the meme prompt page.
Credit
Luis Pabon (they/he)